Wait for API pod to be healthy before registering control plane node in instance group by bochengchu · Pull Request #1533 · kubernetes-sigs/cluster-api-provider-gcp

bochengchu · 2025-09-24T19:30:28Z

/kind bug

What this PR does / why we need it:
Fixes #1407

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #

Special notes for your reviewer:

Please confirm that if this PR changes any image versions, then that's the sole change this PR makes.

TODOs:

squashed commits
includes documentation
adds unit tests

Release note:

Wait for API pod to be healthy before registering control plane node in instance group

netlify · 2025-09-24T19:30:34Z

✅ Deploy Preview for kubernetes-sigs-cluster-api-gcp ready!

Name	Link
🔨 Latest commit	`da844bc`
🔍 Latest deploy log	https://app.netlify.com/projects/kubernetes-sigs-cluster-api-gcp/deploys/6972c1ab15af2f0008586138
😎 Deploy Preview	https://deploy-preview-1533--kubernetes-sigs-cluster-api-gcp.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

k8s-ci-robot · 2025-09-24T19:30:36Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: bochengchu
Once this PR has been reviewed and has the lgtm label, please assign chrischdi for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

k8s-ci-robot · 2025-09-24T19:30:38Z

Hi @bochengchu. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

justinsb · 2025-09-25T20:31:41Z

+				return err
+			}
+		} else {
+			log.V(2).Info("[DEBUG] Skipping registering control plane instance in the instancegroup because machine is not yet provisioned", "name", instance.Name)


Some comments with more background on the problem would be helpful for our future selves I think!

justinsb · 2025-09-25T20:31:56Z

 		Watches(
 			&clusterv1.Machine{},
 			handler.EnqueueRequestsFromMapFunc(util.MachineToInfrastructureMapFunc(infrav1.GroupVersion.WithKind("GCPMachine"))),
+			builder.WithPredicates(predicate.Funcs{


I don't think this is necessary?

justinsb · 2025-09-25T20:32:22Z


-	if err := instances.New(machineScope).Reconcile(ctx); err != nil {
+	instancesSvc := instances.New(machineScope)
+	if err := instancesSvc.Reconcile(ctx); err != nil {


Similarly I don't think this is important to this PR.

justinsb · 2025-09-25T20:33:39Z

Nice catch @bochengchu - I think having a test showing the problem would be awesome (I guess in another PR, and we can figure out the sequencing of merging it). And some comments on the critical line when we don't add the new control plane node until it is ready would be helpful for others looking to understand the code / the issue.

/ok-to-test

bochengchu · 2025-10-14T02:51:26Z

+		return false
+	}
+	for _, condition := range m.Machine.Status.V1Beta2.Conditions {
+		if condition.Type == "APIServerPodHealthy" && condition.Status == "True" {


I think we should look for APIServerPodHealthy to be True here, but this is a v1beta2 condition and v1beta2 seems not yet fully migrated to in this repo, so I hardcoded here. Maybe we can have follow-up work to bring in v1beta2 and update this there?

salasberryfin

Thanks @bochengchu. Can you please add a release note in the PR description?

damdo · 2025-10-24T08:07:53Z

@salasberryfin I see @bochengchu added the note.

justinsb · 2025-11-20T21:56:38Z

/ok-to-test

bochengchu · 2025-11-20T23:32:10Z

/test pull-cluster-api-provider-gcp-build

damdo · 2025-11-21T09:11:36Z

/test pull-cluster-api-provider-gcp-build pull-cluster-api-provider-gcp-e2e-test pull-cluster-api-provider-gcp-make pull-cluster-api-provider-gcp-test

damdo · 2025-11-21T09:16:33Z

I think the PR needs rebasing @bochengchu

instance group

damdo · 2026-03-07T16:11:22Z

/test ?

damdo · 2026-03-07T16:11:49Z

/test pull-cluster-api-provider-gcp-conformance

/test pull-cluster-api-provider-gcp-capi-e2e

egsysoev · 2026-04-23T06:17:39Z


+	// List all machines in the cluster to check if this is the first one.
+	machineList := &clusterv1.MachineList{}
+	if err := r.List(ctx, machineList, client.InNamespace(cluster.Namespace), client.MatchingLabels{clusterv1.ClusterNameLabel: cluster.Name}); err != nil {


If you create the KCP and MachineDeployment at the same time, it won’t work as expected. You should add a filter to select only control plane machines.

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Sep 24, 2025

k8s-ci-robot requested review from damdo and dims September 24, 2025 19:30

k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Sep 24, 2025

k8s-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Sep 24, 2025

bochengchu force-pushed the main branch 2 times, most recently from 3ad69fa to 1c24d1f Compare September 24, 2025 19:34

justinsb reviewed Sep 25, 2025

View reviewed changes

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Sep 25, 2025

justinsb mentioned this pull request Sep 26, 2025

Add an e2e test for only creating an internal load balancer #1536

Merged

bochengchu force-pushed the main branch from 1c24d1f to ec96875 Compare October 14, 2025 02:47

bochengchu commented Oct 14, 2025

View reviewed changes

bochengchu marked this pull request as ready for review October 14, 2025 02:51

k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 14, 2025

k8s-ci-robot requested a review from cpanato October 14, 2025 02:51

bochengchu changed the title ~~Hold adding to instance group until machine is provisioned~~ Wait for API pod to be healthy before registering control plane node in instance group Oct 14, 2025

salasberryfin reviewed Oct 14, 2025

View reviewed changes

k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Oct 15, 2025

bochengchu mentioned this pull request Oct 23, 2025

Add test case for 3 control plane nodes and internal load balancer #1550

Merged

justinsb mentioned this pull request Oct 24, 2025

Minimal MachinePool support #1506

Merged

bochengchu force-pushed the main branch from ec96875 to 12ae31d Compare November 21, 2025 18:44

Wait for API pod to be healthy before registering control plane node in

da844bc

instance group

bochengchu force-pushed the main branch from 12ae31d to da844bc Compare December 7, 2025 22:44

bochengchu force-pushed the main branch from 1b255b8 to da844bc Compare January 23, 2026 00:32

egsysoev reviewed Apr 23, 2026

View reviewed changes

Conversation

bochengchu commented Sep 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

netlify Bot commented Sep 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for kubernetes-sigs-cluster-api-gcp ready!

Uh oh!

k8s-ci-robot commented Sep 24, 2025

Uh oh!

k8s-ci-robot commented Sep 24, 2025

Uh oh!

justinsb Sep 25, 2025

Choose a reason for hiding this comment

Uh oh!

justinsb Sep 25, 2025

Choose a reason for hiding this comment

Uh oh!

justinsb Sep 25, 2025

Choose a reason for hiding this comment

Uh oh!

justinsb commented Sep 25, 2025

Uh oh!

bochengchu Oct 14, 2025

Choose a reason for hiding this comment

Uh oh!

salasberryfin left a comment

Choose a reason for hiding this comment

Uh oh!

damdo commented Oct 24, 2025

Uh oh!

justinsb commented Nov 20, 2025

Uh oh!

bochengchu commented Nov 20, 2025

Uh oh!

damdo commented Nov 21, 2025

Uh oh!

damdo commented Nov 21, 2025

Uh oh!

damdo commented Mar 7, 2026

Uh oh!

damdo commented Mar 7, 2026

Uh oh!

egsysoev Apr 23, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

bochengchu commented Sep 24, 2025 •

edited

Loading

netlify Bot commented Sep 24, 2025 •

edited

Loading